feat: add button group, command, kbd, select, search, and skeleton#248
feat: add button group, command, kbd, select, search, and skeleton#248
Conversation
kylengn
requested review from
EnzalRad,
dvxam and
noe-charmet
as code owners
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughIntroduces six new UI components (ButtonGroup, Command, Kbd, Search, Select, Skeleton) to the React UI library with comprehensive Storybook stories, custom scrollbar CSS styling, and updates the public component exports. Adds cmdk as a dependency. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
libs/react/ui/index.css (1)
972-1018: LGTM! Well-structured scrollbar implementation.The custom scrollbar styles are properly implemented with:
- Cross-browser support using both standard properties and WebKit prefixes
- Consistent use of design tokens for theming
- Proper dark mode adjustments
- Clean hidden variant for cases where scrollbars should not be visible
The implementation follows CSS best practices and integrates well with the existing design system.
Optional: Consider scrollbar width for accessibility
The 4px scrollbar width/height is quite minimal. If users report difficulty interacting with scrollbars, consider increasing to 6-8px for improved accessibility, particularly for users with motor impairments.
.scrollbar::-webkit-scrollbar { - width: 4px; - height: 4px; + width: 6px; + height: 6px; }This is purely optional and depends on your design requirements and user feedback.
libs/react/ui/src/components/kbd/kbd.tsx (1)
4-4: Consider exporting KbdProps for better developer experience.While consumers can use
ComponentProps<'kbd'>directly, exportingKbdPropswould improve consistency and make it easier for developers to extend or wrap the Kbd component with proper typing.🔎 Apply this diff to export KbdProps:
-type KbdProps = ComponentProps<'kbd'>; +export type KbdProps = ComponentProps<'kbd'>;libs/react/ui/src/components/search/search.stories.tsx (1)
40-61: Consider extractingInlineDemooutside the render function.Defining
InlineDemoinside therenderfunction causes it to be recreated on every render, which can lead to state loss and remounting issues. While this may work for simple Storybook demos, extracting it to the module level (likeModalSearchDemo) would be more consistent with the pattern used elsewhere in this file.🔎 Suggested refactor
+function InlineDemo() { + const [value, setValue] = useState(''); + + return ( + <div className="flex flex-col gap-16 max-w-400"> + <SearchInline + placeholder="Search..." + value={value} + onChange={(e) => setValue(e.target.value)} + onClear={() => setValue('')} + /> + {value && ( + <p className="text-sm text-foreground-neutral-muted">Searching for: "{value}"</p> + )} + </div> + ); +} + export const Inline: Story = { - render: () => { - function InlineDemo() { - const [value, setValue] = useState(''); - - return ( - <div className="flex flex-col gap-16 max-w-400"> - <SearchInline - placeholder="Search..." - value={value} - onChange={(e) => setValue(e.target.value)} - onClear={() => setValue('')} - /> - {value && ( - <p className="text-sm text-foreground-neutral-muted">Searching for: "{value}"</p> - )} - </div> - ); - } - return <InlineDemo />; - }, + render: () => <InlineDemo />, };libs/react/ui/src/components/search/search.tsx (1)
147-185: Keyboard shortcut handler doesn't account for already-open state.The shortcut handler always calls
setOpen(true)but doesn't toggle the search closed. If this is intentional (only open via shortcut, close via Escape), consider adding a comment to clarify. Also, the handler could benefit from checking if the search is already open to avoid unnecessary state updates.🔎 Optional: toggle behavior or skip if already open
const handleKeyDown = (e: KeyboardEvent) => { const key = shortcutKey.toLowerCase(); const isMetaKey = key.startsWith('meta+') || key.startsWith('cmd+') || key.startsWith('⌘'); const isCtrlKey = key.startsWith('ctrl+'); const targetKey = key.replace(SHORTCUT_KEY_REGEX, ''); if (isMetaKey && e.metaKey && e.key.toLowerCase() === targetKey) { e.preventDefault(); - setOpen(true); + setOpen(!open); // Toggle behavior, or use `setOpen(true)` if open-only is intended } else if (isCtrlKey && e.ctrlKey && e.key.toLowerCase() === targetKey) { e.preventDefault(); - setOpen(true); + setOpen(!open); } else if (
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (22)
libs/react/ui/index.css(1 hunks)libs/react/ui/package.json(1 hunks)libs/react/ui/src/components/button-group/button-group.stories.tsx(1 hunks)libs/react/ui/src/components/button-group/button-group.tsx(1 hunks)libs/react/ui/src/components/button-group/index.ts(1 hunks)libs/react/ui/src/components/code-block/code-block-footer.tsx(2 hunks)libs/react/ui/src/components/command/command.stories.tsx(1 hunks)libs/react/ui/src/components/command/command.tsx(1 hunks)libs/react/ui/src/components/command/index.ts(1 hunks)libs/react/ui/src/components/index.ts(2 hunks)libs/react/ui/src/components/kbd/index.ts(1 hunks)libs/react/ui/src/components/kbd/kbd.stories.tsx(1 hunks)libs/react/ui/src/components/kbd/kbd.tsx(1 hunks)libs/react/ui/src/components/search/index.ts(1 hunks)libs/react/ui/src/components/search/search.stories.tsx(1 hunks)libs/react/ui/src/components/search/search.tsx(1 hunks)libs/react/ui/src/components/select/index.ts(1 hunks)libs/react/ui/src/components/select/select.stories.tsx(1 hunks)libs/react/ui/src/components/select/select.tsx(1 hunks)libs/react/ui/src/components/skeleton/index.ts(1 hunks)libs/react/ui/src/components/skeleton/skeleton.stories.tsx(1 hunks)libs/react/ui/src/components/skeleton/skeleton.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*
⚙️ CodeRabbit configuration file
We handle errors at the edge of our applications in most cases. Do not recommend to add error handling around every single function. We prefer them to bubble up and be handled at upper layers.
Files:
libs/react/ui/src/components/kbd/index.tslibs/react/ui/src/components/index.tslibs/react/ui/src/components/command/index.tslibs/react/ui/src/components/search/index.tslibs/react/ui/src/components/button-group/index.tslibs/react/ui/src/components/kbd/kbd.tsxlibs/react/ui/package.jsonlibs/react/ui/src/components/kbd/kbd.stories.tsxlibs/react/ui/src/components/skeleton/index.tslibs/react/ui/src/components/skeleton/skeleton.tsxlibs/react/ui/src/components/skeleton/skeleton.stories.tsxlibs/react/ui/src/components/command/command.stories.tsxlibs/react/ui/src/components/code-block/code-block-footer.tsxlibs/react/ui/index.csslibs/react/ui/src/components/search/search.stories.tsxlibs/react/ui/src/components/button-group/button-group.tsxlibs/react/ui/src/components/select/select.tsxlibs/react/ui/src/components/command/command.tsxlibs/react/ui/src/components/button-group/button-group.stories.tsxlibs/react/ui/src/components/select/select.stories.tsxlibs/react/ui/src/components/select/index.tslibs/react/ui/src/components/search/search.tsx
🧬 Code graph analysis (6)
libs/react/ui/src/components/kbd/kbd.stories.tsx (1)
libs/react/ui/src/components/kbd/kbd.tsx (2)
Kbd(6-20)KbdGroup(24-32)
libs/react/ui/src/components/skeleton/skeleton.stories.tsx (1)
libs/react/ui/src/components/skeleton/skeleton.tsx (1)
Skeleton(6-14)
libs/react/ui/src/components/command/command.stories.tsx (3)
libs/react/ui/src/components/command/command.tsx (9)
Command(248-248)CommandInput(251-251)CommandList(252-252)CommandEmpty(253-253)CommandGroup(254-254)CommandItem(255-255)CommandSeparator(256-256)CommandShortcut(257-257)CommandTrigger(249-249)libs/react/ui/src/components/icon/icon.tsx (1)
Icon(86-90)libs/react/ui/src/components/popover/popover.tsx (3)
Popover(60-60)PopoverTrigger(60-60)PopoverContent(60-60)
libs/react/ui/src/components/command/command.tsx (2)
libs/react/ui/src/components/icon/icon.tsx (1)
Icon(86-90)libs/react/ui/src/components/kbd/kbd.tsx (1)
Kbd(6-20)
libs/react/ui/src/components/select/select.stories.tsx (1)
libs/react/ui/src/components/select/select.tsx (8)
Select(207-207)SelectTrigger(210-210)SelectValue(209-209)SelectContent(211-211)SelectItem(213-213)SelectGroup(208-208)SelectLabel(212-212)SelectSeparator(214-214)
libs/react/ui/src/components/search/search.tsx (2)
libs/react/ui/src/components/icon/icon.tsx (1)
Icon(86-90)libs/react/ui/src/components/kbd/kbd.tsx (1)
Kbd(6-20)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Agent
- GitHub Check: Continuous integration
🔇 Additional comments (32)
libs/react/ui/src/components/code-block/code-block-footer.tsx (1)
5-5: Theme-aware loader implementation looks good.The integration of
useResolvedThemeto conditionally style theShipfoxLoaderbased on the active theme is clean and follows React best practices. The hook properly returns'light' | 'dark', and the conditional logic in lines 37-38 correctly adapts the loader's appearance. No error handling is needed here as theme resolution is deterministic and errors would bubble up at a higher layer per your guidelines.libs/react/ui/src/components/button-group/index.ts (1)
1-1: LGTM!Standard barrel export pattern that enables consolidated imports for the ButtonGroup component family.
libs/react/ui/src/components/skeleton/index.ts (1)
1-1: LGTM!Standard barrel export pattern for the Skeleton component module.
libs/react/ui/src/components/skeleton/skeleton.tsx (1)
1-14: LGTM!Clean and minimal Skeleton component implementation with proper type safety, className merging, and prop forwarding. The use of
data-slot="skeleton"provides a consistent identification pattern.libs/react/ui/src/components/skeleton/skeleton.stories.tsx (1)
1-178: LGTM!Comprehensive Storybook coverage demonstrating diverse Skeleton use cases including basic shapes, card loading, list loading, table loading, form loading, and avatar group loading. Well-organized and clearly labeled stories.
libs/react/ui/src/components/button-group/button-group.tsx (1)
1-111: LGTM!Well-architected ButtonGroup component family with proper accessibility (role="group", aria-hidden), flexible composition (Slot support), and comprehensive variant management via CVA. The complex CSS selectors for child styling are appropriate for this use case, and the biome-ignore comment on Line 52 is justified as
role="group"is semantically correct for button groups.libs/react/ui/src/components/button-group/button-group.stories.tsx (1)
1-373: LGTM!Excellent Storybook coverage with comprehensive examples demonstrating ButtonGroup usage across multiple scenarios. Good accessibility practices with consistent
aria-labelattributes throughout, and helpful real-world examples via SelectExample and PopoverExample components.libs/react/ui/src/components/kbd/index.ts (1)
1-1: LGTM!Standard barrel export pattern for the Kbd component module.
libs/react/ui/package.json (1)
41-41: Version ^1.1.1 of cmdk is current and secure.The latest version of cmdk is 1.1.1, and there are no known vulnerabilities. The caret version constraint appropriately allows for patch-level updates while maintaining stability.
libs/react/ui/src/components/select/index.ts (1)
1-1: LGTM!Standard barrel export pattern that aligns with the project's component organization.
libs/react/ui/src/components/command/index.ts (1)
1-1: LGTM!Standard barrel export pattern that aligns with the project's component organization.
libs/react/ui/src/components/search/index.ts (1)
1-1: LGTM!Standard barrel export pattern that aligns with the project's component organization.
libs/react/ui/src/components/index.ts (1)
5-29: LGTM!All six new component exports (
button-group,command,kbd,search,select,skeleton) are correctly positioned in alphabetical order and follow the established barrel export pattern.libs/react/ui/src/components/kbd/kbd.stories.tsx (1)
1-64: LGTM!Comprehensive Storybook coverage demonstrating the Kbd component in multiple contexts:
- Individual keys
- Key combinations with KbdGroup
- Menu integration with shortcuts
The stories effectively showcase the component's versatility and usage patterns.
libs/react/ui/src/components/command/command.stories.tsx (1)
1-133: LGTM!Both stories effectively demonstrate Command component usage:
- Default: Full-featured command palette with search, groups, icons, and shortcuts
- Combobox: Real-world implementation showing state management, popover integration, and selection handling
The
as constassertion on icon names (lines 73-79) ensures type safety, and the combobox logic correctly handles selection state and UI updates.libs/react/ui/src/components/kbd/kbd.tsx (2)
6-20: LGTM!The Kbd component is well-implemented with comprehensive styling, including:
- Proper keyboard key aesthetics
- Theme-aware styling with dark mode support
- Tooltip context-aware variants
- SVG sizing handling
24-32: LGTM!The KbdGroup component provides a clean container for grouping related keyboard keys with appropriate spacing.
libs/react/ui/src/components/select/select.stories.tsx (1)
1-162: LGTM!Comprehensive Storybook coverage demonstrating the Select component's full API surface:
- Basic usage with placeholder
- Grouped options with labels and separators
- Icon-enhanced items for visual context
- Size variants (small, base)
- Style variants (base, component)
The stories provide clear examples for common Select patterns and customization options.
libs/react/ui/src/components/search/search.stories.tsx (2)
141-194: LGTM!The
ModalSearchDemocomponent is well-structured with proper state management, keyboard shortcut configuration, and a comprehensive demonstration of the modal search flow including groups, items, and footer.
257-403: LGTM!The
AllCombinationsstory provides excellent documentation coverage by showcasing all variant permutations (primary/secondary, squared/rounded, base/small) for both inline and modal trigger modes in a clear grid layout.libs/react/ui/src/components/search/search.tsx (3)
469-491: LGTM!The
SearchItemcomponent is well-structured with proper styling for selection states, disabled states, and flexible content layout with optional icon and description support.
535-563: LGTM!The exports are comprehensive and well-organized, exposing all necessary components, variants, types, and the context hook for external use.
397-419: TheautoFocus={open}pattern used here should work reliably in practice. While autoFocus within animated components can theoretically cause timing issues,SearchInputis not a direct child ofAnimatePresence—it's nested insideSearchContent'smotion.divcontainer. Since the input element itself is not animated and only appears as a result of the parent container's animation, focus management should not be disrupted. No action needed unless focus timing issues are observed in practice.libs/react/ui/src/components/select/select.tsx (4)
7-17: LGTM!Clean wrapper components that properly forward props and add
data-slotattributes for styling hooks.
159-188: LGTM!The
SelectItemcomponent handles icon positioning well with absolute positioning, proper spacing adjustments via conditionalpl-56/pl-32classes, and includes the item indicator for selected state.
206-220: LGTM!Exports are comprehensive, including all components, variant definition, and type exports.
126-135: Syntax is supported in Tailwind v4.The parentheses syntax
h-(--radix-select-trigger-height)andmin-w-(--radix-select-trigger-width)is official Tailwind v4 syntax for CSS variables. Confirm your project uses Tailwind v4; if using v3, convert toh-[var(--radix-select-trigger-height)]instead.libs/react/ui/src/components/command/command.tsx (5)
126-132: Escape key handler may interfere with cmdk's built-in behavior.The
handleKeyDownwithe.stopPropagation()on Escape (line 129) prevents the event from bubbling. This could interfere with cmdk's built-in keyboard handling if it relies on the event propagating. Consider whether stopping propagation is necessary here, or if simply preventing default and clearing is sufficient.
41-64: LGTM!The
CommandTriggercomponent is well-implemented with proper ref forwarding, placeholder/value display logic, and consistent styling via CVA variants.
187-202: Consistent implementation with SearchGroup.The
CommandGroupstyling uses the same**:[[cmdk-group-heading]]pattern asSearchGroupin search.tsx, which is good for consistency across the codebase.
247-261: LGTM!Exports are comprehensive and well-organized, including all components, variant definition, and type exports.
80-89: CustomDialog backdrop handling is already managed by CommandPrimitive.Dialog.The custom backdrop div serves only for styling (background color and blur effect). cmdk's Dialog component wraps CommandPrimitive using Radix UI's Dialog, which includes its own overlay for handling backdrop interactions. Radix Dialog automatically provides focus trapping and closes on Escape key press, so no additional onClick handler is needed. Per the coding guidelines, behavior should bubble up through the underlying primitive rather than being duplicated at custom wrapper levels.
Adding
aria-hidden="true"to the visual backdrop is reasonable for accessibility purposes.
There was a problem hiding this comment.
Pull request overview
This PR adds six new UI components to the React UI library: ButtonGroup, Command, Kbd, Select, Search, and Skeleton. The changes include adding the cmdk package as a dependency, implementing comprehensive component implementations with Storybook stories, and adding custom scrollbar styles. Additionally, the CodeBlockFooter component is updated to support theme-aware loading indicators.
Key Changes
- Added six new components (ButtonGroup, Command, Kbd, Select, Search, Skeleton) with full TypeScript support and Storybook documentation
- Integrated
cmdkv1.1.1 library for command palette and search functionality - Added custom scrollbar styling with theme support to index.css
- Updated CodeBlockFooter to use theme-aware ShipfoxLoader colors
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Added cmdk v1.1.1 dependency and removed CPU architecture constraints from Biome CLI packages |
| libs/react/ui/package.json | Added cmdk dependency to the package |
| libs/react/ui/src/components/skeleton/* | New Skeleton component for loading state placeholders with comprehensive story examples |
| libs/react/ui/src/components/select/* | New Select dropdown component built on Radix UI with variants and icon support |
| libs/react/ui/src/components/search/* | New Search component with inline and modal variants, keyboard shortcuts, and filtering |
| libs/react/ui/src/components/kbd/* | New Kbd component for displaying keyboard shortcuts with group support |
| libs/react/ui/src/components/command/* | New Command palette component with search, filtering, and keyboard navigation |
| libs/react/ui/src/components/button-group/* | New ButtonGroup component for grouping related buttons with horizontal/vertical orientation |
| libs/react/ui/src/components/index.ts | Updated to export all new components |
| libs/react/ui/src/components/code-block/code-block-footer.tsx | Updated to use theme-aware loader colors based on resolved theme |
| libs/react/ui/index.css | Added custom scrollbar styles with theme support and hidden scrollbar variant |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
libs/react/ui/src/components/button-group/button-group.stories.tsx
Outdated
Show resolved
Hide resolved
…ling and functionality; add search context and related components
2 participants
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.